Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual Conversion #18588

Merged
merged 95 commits into from
Dec 7, 2016
Merged

Conversation

MichaelHatherly
Copy link
Member

@MichaelHatherly MichaelHatherly commented Sep 19, 2016

This moves the manual to markdown and uses Documenter.jl to generated the HTML docs.

TODO items:

  • decide how to deploy the generated docs;
  • integrate doc/make.jl into the Makefiles;
  • move/remove the rest of the files in the doc/ directory;
  • check that the generated docs aren't broken.

I've opened the branch for review now since it is at the point where further changes will need to be done manually and will become difficult to rebase (these changes touch most of doc/ and base/) easily.

My suggestion would be to check that the converted docs haven't lost any information, all links still point to the correct places, and address the other TODO items above. After that has all been verified then merge this branch and further polishing and rearranging and/or restructuring of the manual can take place with less chance of running into merge conflicts.

Currently the "doc checks" part of the Documenter build is disabled. This includes checks such as doctests, missing docs, and missing/duplicate footnote checks. Fixing the warnings currently being raised when enabling these checks will each need individual attention, but can be split over several PRs to make reviewing easier. In my mind these checks are only absolutely necessary for a released version of Julia, and for master we can probably manage without them for a couple of days. If anyone feels that this isn't a good trade-off then I'll fix everything in this PR, but it may take a while longer to get it done properly.

Here's a link the to generated docs to make reviewing a bit easier: https://michaelhatherly.github.io/julia-docs/. Note that source links for docstrings will 404 since the commits they point to aren't on master.

@MichaelHatherly MichaelHatherly added docs This change adds or pertains to documentation docsystem The documentation building system labels Sep 19, 2016
@@ -29,7 +29,7 @@ end
"""
@enum EnumName EnumValue1[=x] EnumValue2[=y]

Create an [`Enum`](:obj:`Enum`) type with name `EnumName` and enum member values of
Create an `Enum` type with name `EnumName` and enum member values of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there going to be autodetection of some kind for references like these?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enum itself doesn't actually have a docstring, only @enum does. For some reason sphinx didn't ever complain about that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: autodetection, I think we can probably add, at some point, a check to Documenter that will suggest non-references that could be references.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Was this the case for all of the ones you removed in this commit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there's a whole bunch that aren't links any more: Integer, Real, Rational, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And none of those types have docstrings?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly for UInt, Int, Float16 types. I think seems like this could be fixed in a later PR. Linking every type_name automatically seems like a neat Documenter feature!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those ones as well. What I'll do is put together a list of which ones are missing after this conversion is done and open an up-for-grabs issue to track which ones still need documenting.

@tkelman
Copy link
Contributor

tkelman commented Sep 19, 2016

I'm hoping 43f05c2 was automated. Is there a script you have that did that? Would it be possible to separate that huge commit into one commit per modified file, so there's a way to get github to show the whole diff (one piece at a time)?

@MichaelHatherly
Copy link
Member Author

MichaelHatherly commented Sep 19, 2016

Yes, pretty much all automated. I should be able to do one commit each I think. (Seems like that worked...)

@hayd
Copy link
Member

hayd commented Sep 20, 2016

It would be nice if there were some line breaks in the long-lines. (Can be added later.)

closes #13308 #12573 #13047

Sterling work! 👏

Edit: I looked through 20 or so pages and they all looked great / I couldn't see anything amiss.

@MichaelHatherly
Copy link
Member Author

MichaelHatherly commented Sep 20, 2016

It would be nice if there were some line breaks in the long-lines.

I'm inclined to keep the paragraphs in the .md files soft-wrapped rather than hard-wrapped at 92 chars. The reason being that hard-wrapped paragraphs end up being re-wrapped when changes are made which makes the diffs difficult to read, whereas single line paragraphs should shouldn't have that problem as far as I can tell. (For docstrings in .jl files we should still hard-wrap (apart from things like tables) them to match the surrounding source and avoid having to scroll sideways when checking diffs.)

@tkelman
Copy link
Contributor

tkelman commented Sep 20, 2016

It can be a bit annoying to look at doc diffs of super long soft-wrapped lines, since you get the whole thing before then the whole thing after, and it's tough to spot specific changes. Github does a better job highlighting short-line diffs than long-line diffs from what I've seen, I personally think a diff with re-wrapping the start and end of a bunch of lines is easier to check than a diff where you can't tell what actually changed.

@MichaelHatherly
Copy link
Member Author

I can see if hard-wrapping them looks better, will be later this evening though.

end

and not a ``x`` in the scope where ``foo`` is used::
# [Scope of Variables](@id scope-of-variables)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this file and no text has been lost, all links work. There are some minor formatting issues, which I will report when requested. I noted that there is no highlighting in julcon blocks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jlcon is being used for all code blocks containing Julia REPLs since plain julia ends up highlighting results incorrectly sometimes, such as

julia> x -> x
(::#1) (generic function with 1 method)

jlcon is used in the pygments highlighter to correctly highlighting Julia REPL blocks, so there's precedence for that naming convention. We're using highlight.js for highlighting, which I hope we can add an equivalent jlcon highlighter to eventually.

What were the other formatting issues you encountered?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section uses mixed REPL non-REPL blocks (which may not be clever although readable), see e.g. https://michaelhatherly.github.io/julia-docs/manual/variables-and-scoping.html#Local-Scope-1, those used to be highlighted better.

The table right at the start was nicer before with sub-tables but no big deal.

https://michaelhatherly.github.io/julia-docs/manual/variables-and-scoping.html#Hard-Local-Scope-1: the second paragraph starting with "In a hard local scope, ..." was some kind of block & indented. The purpose was to make it stand out more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section uses mixed REPL non-REPL blocks (which may not be clever although readable)

Yes, mixed blocks seems, to me, like something we should avoid for the sake of consistency. (Note, we could just highlight all blocks as Julia code and get back to the same highlighting as before I think.)

The table right at the start was nicer before with sub-tables but no big deal.

Bit unfortunate, but markdown doesn't have a standard syntax for sub-tables, so that was the best approximation I could come up with. I would like to eventually add sub-tables, probably based off the pandoc ones.

the second paragraph starting with "In a hard local scope, ..." was some kind of block & indented. The purpose was to make it stand out more.

Seems the blockquote wasn't converted correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, yes, maybe use Julia-highlighting for now.

@MichaelHatherly
Copy link
Member Author

Hard-wrapped paragraphs, fixed the blockquote issue @mauro3 found, and now using julia as the highlighter for all REPL code blocks rather than jlcon.

@MichaelHatherly MichaelHatherly force-pushed the mh/markdown-docs branch 2 times, most recently from 0fbc4c3 to 54447f5 Compare September 21, 2016 09:40
@MichaelHatherly MichaelHatherly force-pushed the mh/markdown-docs branch 2 times, most recently from c48d043 to 48731a5 Compare September 23, 2016 09:49
@#Check documentation
@$(JULIA_EXECUTABLE) $(JULIAHOME)/doc/NEWS-update.jl #Add missing cross-references to NEWS.md
@$(MAKE) -C $(BUILDROOT)/doc unicode #Rebuild Unicode table if necessary
@$(JULIA_EXECUTABLE) $(JULIAHOME)/doc/DocCheck.jl > $(BUILDROOT)/doc/UNDOCUMENTED.rst 2>&1 #Check for undocumented items
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we port this or is this totally bitrotted and useless right now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documenter provides pretty much the same features for checking for missing docs. Should be fine to remove.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this never actually did anything since it's just defining a module but not running any of the code it defines

@$(MAKE) -C $(BUILDROOT)/doc html SPHINXOPTS="-n" #Rebuild Julia HTML docs pedantically
@$(MAKE) -C $(BUILDROOT)/doc latex SPHINXOPTS="-n" #Rebuild Julia PDF docs pedantically
@$(MAKE) -C $(BUILDROOT)/doc doctest #Run Julia doctests
@$(MAKE) -C $(BUILDROOT)/doc linkcheck #Check all links
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pdf, doctest, and linkcheck all need to run in release-candidate. In strict mode if Documenter is going to differentiate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently there's no PDF output option for Documenter, so if that's deemed important enough to have working as soon as we switch over then this PR will need to wait until I've got some time to implement and properly test it first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.6rc is kinda a long way off, surely that doesn't need to hold up this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pdf is pretty widely used as I understand it, but not necessarily from master. It's okay for it to be missing from master for a few weeks, but any longer than that would not be okay as the plan is actually to do 0.6 on a very abbreviated schedule if we can.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot gate the transition on being 100% feature complete. If someone wants the PDF badly enough, they can implement that feature in Documenter.

Copy link
Contributor

@tkelman tkelman Sep 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but we can't afford to be in a state that we can't release from for very long. Not providing a pdf is a pretty big regression, the pdf form of the manual is the way many people start learning Julia.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'll do is see whether I can get a latex/pdf output sorted during the next week. It's definitely not going to need as much work as the html, so it might very well be doable. (Hopefully we aren't wanting any other formats, are we?)

At the moment this branch is really trivial to rebase over any changes, so I'm not too worried about rushing it in if there's things that can be fixed beforehand that would make the transition better.

(Out of interest does readthedocs provide any stats on number of downloads of the pdf vs number of website hits?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also: JuliaDocs/Documenter.jl#4. If anyone has any particular requests for how they'd like the pdf docs to be styled/structured then comments over there are very welcome. I'll probably try to follow the current sphinx style reasonably closely where possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For those not following, @MichaelHatherly added pdf support to Documenter: https://dl.dropboxusercontent.com/u/14501513/julia.pdf

It seems like the key question remaining here is:

decide how to deploy the generated docs;


@# Check to see if the above make invocations changed anything important
@if [ -n "$$(git status --porcelain)" ]; then \
echo "Git repository dirty; Verify and commit changes to the repository, then retry"; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put this back


deps:
@echo "Installing documentation dependencies."
$(JULIA_EXECUTABLE) -e 'Pkg.installed("Documenter") === nothing && Pkg.add("Documenter")'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't modify global package state - better to put a REQUIRE file in here and set JULIA_PKGDIR while running the commands for this

@MichaelHatherly
Copy link
Member Author

(Apologies for the delays in finishing this up, was travelling. I'll try finish it off over the weekend.)

@MichaelHatherly
Copy link
Member Author

Rebased, some makefile changes for doctest/linkcheck targets, and install deps in a separate JULIA_PKGDIR to avoid using the user's package dir.

The generated docs available here:

unicode: $(BUILDDIR)/manual/unicode-input-table.rst
doctest:
@echo "Running all embedded 'doctests'."
$(JULIA_EXECUTABLE) make.jl -- doctest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a strict mode yet for checking releases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just yet, I'll add one shortly.

@CarloLucibello
Copy link
Contributor

looks like paragraph division got lost here https://michaelhatherly.github.io/julia-docs/devdocs/reflection.html

@MichaelHatherly
Copy link
Member Author

looks like paragraph division got lost here https://michaelhatherly.github.io/julia-docs/devdocs/reflection.html

Thanks, those are .. rubric:: directives it seems. There's nothing equivalent in Markdown, so I'll probably just switch them to ## headers which should match the page layout alright I think.

@MichaelHatherly
Copy link
Member Author

This fails on the Windows buildbots

Are there notable differences between the Windows buildbots and Appveyor? Output redirection has worked pretty well there for a while. I guess we could just disable doctests when building there, or perhaps just only build on the Linux ones since the docs should really be identical no matter where they were built and reuse those elsewhere.

This also broke the RPM nightlies

Not sure what to make of that error,

make[2]: /builddir/build/BUILD/julia/usr/bin/julia: Command not found

since I believe I used the command path as was already in use. Is there a better way to get the path to the julia executable?

@tkelman
Copy link
Contributor

tkelman commented Dec 8, 2016

Appveyor doesn't run make install or run the build process under output redirection, the buildbots are driven by a python program. make install includes the html docs, so that needs to work.

@MichaelHatherly
Copy link
Member Author

Are there any workarounds for #11727? I could possibly hack something into Documenter to just disable anything that uses any redirection, bit of a heavy handed solution though.

@tkelman
Copy link
Contributor

tkelman commented Dec 8, 2016

Redirecting to a file might work, or suppressing the doc output when run under make install. Preferably fix the issue, if @vtjnash would either do so or explain how less cryptically.

@nalimilan
Copy link
Member

since I believe I used the command path as was already in use. Is there a better way to get the path to the julia executable?

Not sure what changed, but that path wasn't correct in general. See #19539.

Though even without that change, there's another problem to build RPMs: Internet access isn't available on build machines. So I'll need to find a way to download the packages beforehand. We should perform this step automatically from make dist so that distributions and users can build docs from the official source without downloading anything else.

(Note that these complaints are minor compared to the great improvement made by this PR.)

``:push_loc``: enters a sequence of statements from a specified source location.
- ``args[2]`` specifies a filename, as a symbol.
- ``args[3]`` optionally specifies the name of an (inlined) function that originally contained the code.
* `:push_loc`: enters a sequence of statements from a specified source location.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this didn't previously have a bullet, though the inline and pop_loc lines probably should instead of the way it was

Copy link
Member Author

@MichaelHatherly MichaelHatherly Dec 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that page is a bit messy due to use of rst definition lists which our markdown doesn't really have an equivalent for. I'll try tidy things up a bit there.

@MichaelHatherly MichaelHatherly mentioned this pull request Dec 9, 2016
@MichaelHatherly
Copy link
Member Author

Though even without that change, there's another problem to build RPMs: Internet access isn't available on build machines. So I'll need to find a way to download the packages beforehand. We should perform this step automatically from make dist so that distributions and users can build docs from the official source without downloading anything else.

I suppose that kind of ties into whatever we decide to do with #18795.

@nalimilan
Copy link
Member

Actually, I would be both more logical and easier to build the docs during the make dist step so that make install can skip this step when building from a tarball.

@mauro3
Copy link
Contributor

mauro3 commented Dec 9, 2016

Should the doc-links on julialang.org be udated? Or are the new docs still "secret"?

@MichaelHatherly
Copy link
Member Author

Should the doc-links on julialang.org be udated?

Yes, links need to be updated. Has to be done by someone with access to the domain. @StefanKarpinski would you be able to sort that out when you've got a spare moment? Thanks.

Actually, I would be both more logical and easier to build the docs during the make dist step so that make install can skip this step when building from a tarball.

Is it actually necessary any more to build the docs on any of the buildbots? When we were using Sphinx it did make sense since readthedocs built slightly different docs to a local sphinx build. Documenter's builds are all exactly the same wherever they are built, so we could potentially just download the files from gh-pages instead.

@nalimilan
Copy link
Member

Documenter's builds are all exactly the same wherever they are built, so we could potentially just download the files from gh-pages instead.

We could, but then people wouldn't be able to generate custom tarballs after modifying the source. Not a big deal, but I prefer generating documentation on the fly to match exactly the tarball which is generated than rely on an external documentation source. Anyway, the fix was pretty simple, I've added it to #19539.

@MichaelHatherly
Copy link
Member Author

We could, but then people wouldn't be able to generate custom tarballs after modifying the source.

Would they not then just run make docs after customising? Or are we needing to support building a modified source completely offline? Anyway, no need if you've already got a fix, thanks.

@Ken-B
Copy link
Contributor

Ken-B commented Dec 11, 2016

Great work!

Do you consider to provide the documentation also in .epub format? It's been published in epub at least since Julia 0.2, that's how I learned the language which I found very handy at the time.

@MichaelHatherly
Copy link
Member Author

Thanks. epub is reasonably similar to html so it's probably not going to be too much work to add a new backend to Documenter to support it, but unless someone else implements it it'll have to wait a while until I've got some spare time to do it. @Ken-B can you open an issue over on Documenter's tracker so this doesn't get forgotten?

|:------- |:--------- |:-------------------------------- |
| Native | `julia_` | Speed via specialized signatures |
| JL Call | `jlcall_` | Wrapper for generic calls |
| JL Call | `jl_` | Builtins |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does markdown not have a way of making table entries extend across multiple rows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, markdown doesn't usually have that I think, though there are some dialects, such as pandoc, that do seem to support those kind of things, so we could probably just extend our parser as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. In case you hadn't guessed, I've been checking a few pages using http://services.w3.org/htmldiff, looking at a branch on my fork where I pushed the last pre-conversion commit's html docs (https://github.com/tkelman/julia/commits/gh-pages-preconversion). Haven't done too many yet, but the ones I've looked at have been remarkably clean.

We do need to figure something out for the windows buildbots though.

tkelman added a commit that referenced this pull request Dec 16, 2016
with documentation build, ref #18588 (comment)
redirecting to a file should work, but redirecting to a cygwin pipe
or driving application like perl or python (in the buildbot case) doesn't
tkelman added a commit that referenced this pull request Dec 17, 2016
* Work around redirection issue on windows buildbots

with documentation build, ref #18588 (comment)
redirecting to a file should work, but redirecting to a cygwin pipe
or driving application like perl or python (in the buildbot case) doesn't

* Simpler case that's not as windows specific

* Revert "Simpler case that's not as windows specific"

This reverts commit 4424c54.

Unfortunately this doesn't stop on possible failures in the doc build

function unicode_data()
file = normpath(JULIA_HOME, "..", "..", "doc", "UnicodeData.txt")
url = "http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downloading this list from an external source doesn't sound like a good idea to me. That means the data can change without Julia supporting newly-introduced forms (since IIUC this depends on the utf8proc version we use). How about adding this file to git? Or couldn't we get this information from utf8proc itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, probably would be better to use a stable source for it. With this rewrite I just made sure it was doing the same thing as before rather than going and changing anything like that.

Or couldn't we get this information from utf8proc itself?

The file is only used to lookup the unicode names for chars, so if utf8proc can do that then that would be great.

Copy link
Member

@Sacha0 Sacha0 Dec 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downloading this file also seems to cause build failures, e.g. https://travis-ci.org/JuliaLang/julia/builds/186607980 ? Best!

Edit: Seems that download attempt has been causing build failures regularly since last night?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, guess we should just commit that file then (though it's a bit big perhaps), or maybe host it ourselves somewhere more reliable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevengj Your opinion on this?

Copy link
Member

@stevengj stevengj Dec 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utf8proc doesn't include names for unicode characters, so it won't help with this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't commit it, but we should be specific about what version we download (not latest) and use juliacache where this is probably already mirrored

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would certainly improve the reproducibility of builds. I guess we should also include that file in the tarballs? It would be weird not to be able to build Julia and its documentation offline from the full tarball with dependencies, just because of that missing file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We put the result of the doc build in the tarball, I think it's okay to not include all the inputs if we include the output. Re-generating it isn't terribly hard with a few packages and additional files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.